Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General cleanup #244

Merged

Conversation

phackstock
Copy link
Contributor

@phackstock phackstock commented May 9, 2023

As I ran into an issue with a mutable default argument in #243, this PR eliminates the two remaining ones in the package.

Since this is, on purpose, branched off of #243, the idea would be to first merge #243 and then this one.

Update

I used pylint to find the mutable defaults. While I was there I also fixed some other issues that it found.

@phackstock phackstock added the bug Something isn't working label May 9, 2023
@phackstock phackstock self-assigned this May 9, 2023
@phackstock phackstock changed the title Remove all mutable defaults General cleanup May 9, 2023
@phackstock phackstock force-pushed the bug/remove-all-mutable-defaults branch from 91baf31 to 126a5b2 Compare May 9, 2023 14:18
@danielhuppmann
Copy link
Member

@phackstock, can you please do a rebase before I review?

@phackstock phackstock force-pushed the bug/remove-all-mutable-defaults branch from 126a5b2 to d63dc28 Compare May 17, 2023 14:27
@phackstock
Copy link
Contributor Author

@danielhuppmann everything rebased, good to review now.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, these are a lot of changes, but it all looks pretty straightforward to me...

@phackstock
Copy link
Contributor Author

Yes, nothing major, I just fixed a bunch of stuff that pylint found while I was at it.

@phackstock phackstock merged commit b76b28f into IAMconsortium:main May 17, 2023
@phackstock phackstock deleted the bug/remove-all-mutable-defaults branch May 17, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants